Migrate Yarn 4 PnP to pnpm 11, and repair the quality gates that were hiding - #268
Draft
cooperability wants to merge 18 commits into
Draft
Migrate Yarn 4 PnP to pnpm 11, and repair the quality gates that were hiding#268cooperability wants to merge 18 commits into
cooperability wants to merge 18 commits into
Conversation
Swaps the package manager without changing a single dependency version. The lockfile was produced with `pnpm import`, which reads yarn.lock and preserves its exact resolutions, so this commit changes the mechanism and nothing about what resolves. Version changes land separately. What goes away: - 1,379 committed .yarn/cache zips (778 MB in the working tree). pnpm keeps one copy of each package in a global content-addressable store and hardlinks it into node_modules. - .pnp.cjs, .pnp.loader.mjs and .yarn/sdks. With a real node_modules tree, Jest, ESLint, Next and tsserver resolve natively, so the editor SDK shim layer in .vscode/settings.json is deleted rather than ported. - The .gitignore rules excluding *-win32-* and *-darwin-* cache archives. Those existed only because a developer installing on Windows would otherwise overwrite the linux binaries Vercel needed to deploy. Configuration notes, all of which are pnpm 11 specifics that fail silently if got wrong: - pnpm 11 made .npmrc auth/registry-only. Every other setting lives in pnpm-workspace.yaml. Settings left in .npmrc are ignored without warning. - `allowBuilds` replaced `onlyBuiltDependencies`, which pnpm 11 removed and now ignores. Config copied from any pnpm 10 guide looks right and does nothing. - Yarn's `resolutions` became `overrides`, carried over verbatim. Yarn ran every dependency install script (enableScripts: true). pnpm denies all of them by default and takes an explicit allowlist, so this is a net supply-chain improvement: four packages are allowed to run build scripts, and a compromised release of anything else gets no install-time execution. Also fixes two things the migration made unavoidable: - `build` still carried `ls -la && ls -la .yarn` debug probes from a Vercel investigation; the .yarn probe now hard-fails. - `test` was `jest --watch`. An interactive watcher under the conventional script name is a footgun that CLAUDE.md carried a standing warning about. `test` is now non-interactive; `test:watch` is the watcher. The pre-commit hook has been disabled behind a "Commented out for Windows development" note, with a .bat sibling meant to cover Windows that git never invoked -- git runs .husky/pre-commit through sh on every platform. Both hooks are now single cross-platform scripts and the dead .bat files are gone. Verified: pnpm install (14.9s cold, warm store), typecheck, and a full production build including service worker and sitemap (12.5s) all pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
`pnpm lint` (and `yarn lint` before it) did not lint anything. It crashed.
Confirmed pre-existing: running `yarn lint` on the pre-migration checkout
fails the same way. Nothing caught it because no CI job has ever run lint.
Three separate faults were stacked on top of each other, each hidden by the
one in front:
1. `--ext` was removed in ESLint 9. Flat config decides its own file scope,
so the flag is now an error. Dropped; `lint:mdx` went with it because the
flat config already routes .mdx through the remark processor.
2. `FlatCompat` wrapped `next/core-web-vitals`. eslint-config-next 16 ships a
real flat config array, so translating it through the eslintrc compat
layer threw `Converting circular structure to JSON`. Importing
`eslint-config-next/core-web-vitals` natively removes the shim and
@eslint/eslintrc with it.
3. eslint-config-next was pinned at ^15.4.8 while next is 16.x -- a full
major behind. Bumped to 16.3.2, with typescript-eslint 8.35.1 -> 8.68.0.
ESLint itself is moved 10.0.2 -> 9.39.5, which is a downgrade and is the
correct version. eslint-plugin-react's LATEST release (7.37.5) declares
`peerDependencies: { eslint: "^3 || ... || ^9.7" }` -- it has no ESLint 10
support at all, and eslint-config-next pulls it in transitively. On ESLint 10
it dies in `detectReactVersion` calling `context.getFilename()`, removed in
that major. This repo had moved ahead of the plugin ecosystem.
Revisit when eslint-plugin-react ships an ESLint 10 peer range.
With lint actually running, it found six real problems. Five are
`react-hooks/set-state-in-effect`:
- Four (ActiveIcon, ThemeSwitch, providers, useResponsive) are the canonical
SSR hydration mount-guard, a known false-positive class for this rule. Each
is silenced at the single call site with the reasoning inline. The rule is
deliberately NOT downgraded globally, so a genuine future violation still
fails the build.
- One is real. OpioidConverter holds values DERIVED from `medications` in
state and re-syncs them through an effect, costing a render pass per
keystroke. It is annotated as a known issue rather than fixed: it is a
medical dosing calculator with no test coverage, and it needs
characterisation tests before the arithmetic is touched.
The sixth was an eslint-disable directive in scripts/create-report-dir.js
that no longer suppressed anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
The suite asserted a nav link named /learnings/i pointing at /resources. src/pages/index.tsx renders that link as "knowledge" -- the copy was changed and the test was never updated, so this has been failing on main. Like the lint breakage in the previous commit, it survived because no CI job runs the test suite. Both are fixed here; the CI workflow that would have caught them lands next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Before this commit the repository had exactly one workflow, and it audited dependencies. Nothing ran lint. Nothing ran the test suite. Nothing ran a production build. That is the direct cause of the two previous commits: a crashing ESLint config and a test asserting copy that had been rewritten both sat on main indefinitely, because no automation would ever have noticed. New `ci.yml` runs lint, typecheck, tests-with-coverage and a real production build on every PR and every push to main. - `pnpm/action-setup` deliberately pins no version. It reads `packageManager` from package.json, so CI, Vercel and every developer machine take the pnpm version from one place. - It must run BEFORE setup-node, because `cache: pnpm` asks the pnpm binary where its store is. - `--frozen-lockfile` is the `yarn install --immutable` equivalent: fail rather than quietly resolve something the lockfile never recorded. - Steps are `if: !cancelled()` so one red run reports every failure at once instead of one failure per push. - `concurrency: cancel-in-progress` kills superseded runs; the result of a run against an overwritten commit is not worth paying for. `security-audit.yml` moves to pnpm and drops its install step entirely. `pnpm audit` resolves the tree from pnpm-lock.yaml and queries the advisory API directly, so it needs neither node_modules nor a store. The old workflow ran a full `yarn install --immutable` first, which also meant every dependency install script executed on a runner whose only job was to read a lockfile. Note that pnpm 11 no longer proxies audit through the npm CLI, so the flag is `--audit-level`, not `--severity`. Actions moved to current majors, which folds in Dependabot #237 and #246: checkout v6->v7, setup-node v6->v7, upload-artifact v7, github-script v9. Adds a `test-exclude: ^7.0.2` override, without which the new coverage step cannot run at all. The inherited `glob: ^10.5.0` override reaches test-exclude@6, which calls `util.promisify(glob)`; glob 10 exports an object rather than a function, so instrumentation died with `The "original" argument must be of type function`. test-exclude@7 uses the glob 10 API. This was latent under Yarn too -- coverage had simply never been run. Dependabot needs no ecosystem change: `npm` covers pnpm and it picks the manager from the lockfile. The 20 PRs currently open against yarn.lock cannot rebase onto this tree and must be closed; Dependabot will reopen them against pnpm-lock.yaml. That is left to a human deliberately. Verified locally end to end: lint clean, typecheck clean, 4/4 tests passing with coverage collected, production build succeeds. All four workflow and config YAML files parse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Takes the repository from 53 known advisories to 5. All five that remain have no upstream fix: postcss (4) and sharp (1) are already at the latest published version, so nothing to move to. Resolves the substance of the open Dependabot PRs, which cannot themselves survive this branch -- they carry yarn.lock diffs and cannot rebase onto a tree with no yarn.lock. Direct bumps of note: - next 16.2.6 -> 16.2.11 (#254) - sharp 0.34.5 -> 0.35.3 (#252) - react / react-dom 19.1 -> 19.2.8, @types to match - @serwist/build 9.2.1 -> 9.5.12 (#213) - jest-environment-jsdom 30.2.0 -> 30.4.1 (#215) - postcss 8.5.10 -> 8.5.26 (#257), @babel/core (#241), and the rest of the patch-updates group (#265) via in-range `pnpm update` - @testing-library/jest-dom 6.6.3 -> 7.0.1. pnpm flagged 6.10.0 as a mis-published minor carrying breaking changes; 7.x is the clean line and its @testing-library/dom peer range is already satisfied. Transitive advisories (nanoid #264, brace-expansion #259, fast-uri #258, js-yaml #256, ws #247, form-data #236, tar #229 (the one CRITICAL), minimatch #228, follow-redirects #222) were resolved by letting `pnpm update` move the parents that pin them, rather than by pinning each child directly. That distinction is the point of the one override this adds: socks: ^2.8.9 ip-address had two advisories. The chain is socks -> ip-address, and socks 2.8.5 pins `ip-address: ^9`. Overriding ip-address to ^10 would have handed socks an API it does not expect -- precisely the failure that killed coverage when the inherited glob override reached test-exclude@6. socks 2.8.9 already declares `ip-address: ^10.1.1`, so bumping the parent lets the fixed child arrive legitimately. Fix the package that declares the bad range, not the package named in the advisory. Also drops @eslint/compat and @eslint/eslintrc, which became dead when the FlatCompat layer was removed, and annotates one more real `react-hooks/set-state-in-effect` hit that eslint-plugin-react-hooks 7.1 newly detects in PromptComposer. Like the OpioidConverter one it is described rather than fixed: it changes when a user's manual edits are discarded, and the component has no test coverage. NOT included, deliberately: tailwindcss 3 -> 4 (#214). That is a real migration -- a new config format and a rewritten cascade layer model -- with visual consequences on every page, and it belongs in its own PR with a human looking at the result. Verified: lint clean, typecheck clean, 4/4 tests with coverage, production build 14.2s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
The first preview deploy on this branch failed. The previous, working Yarn configuration set ENABLE_EXPERIMENTAL_COREPACK=1, which is the flag Vercel requires before corepack is usable in the build image; dropping it while adding an explicit `corepack enable && corepack prepare --activate` to the install command was almost certainly the cause, since corepack enable has nothing to attach to without it. Restores the flag and reduces installCommand to plain `pnpm install --frozen-lockfile`, letting Vercel's own corepack support read the `packageManager` field. That keeps one source of truth for the pnpm version across Vercel, CI and developer machines, which matters more here than usual: pnpm 11 reads overrides and allowBuilds from pnpm-workspace.yaml, and older pnpm silently ignores that file, so a host quietly choosing its own pnpm would resolve a different tree without failing. UNVERIFIED. There is no Vercel CLI or token in this environment, so the build log could not be read and this fix is reasoned from the diff, not observed. The next preview deploy is the test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Vercel parses engines.node to choose a runtime and accepts a major selector, not a semver range; ">=22.13.0" fails the build outright. Back to "22.x", which still satisfies pnpm 11's own >=22.13 requirement. Changed alone, deliberately: the preview deploy is the only instrument available here (no Vercel CLI or token in this environment to read a build log), so changing one variable per push is the only way to learn which one matters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
`packages: [.]` declared this single-package repo as a pnpm workspace root. pnpm does not need it -- overrides and allowBuilds are still read, verified by a clean --frozen-lockfile install that applies the socks override and runs the allowed build scripts -- but it makes the repo look like a monorepo to tooling that keys off pnpm-workspace.yaml, Vercel included. Third single-variable attempt at the failing preview deploy. No Vercel CLI or token exists in this environment, so the build log cannot be read and each hypothesis has to be tested by pushing it alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Fourth single-variable attempt at the failing preview deploy, and the first one aimed at the builder rather than the package manager. The App Router branch (#267) explicitly pins `next build --webpack`, which is a deliberate opt-out of the Next 16 default. The most likely reason to add that flag is that Turbopack does not build this project on Vercel -- which would mean the failing deploys here have nothing to do with pnpm at all, and everything to do with this branch restoring the default builder while removing the `ls -la .yarn` debug probes from the same script. If this deploy goes green, the builder was the cause and the pnpm migration was never implicated. That also revises the recommendation in docs/PNPM-MIGRATION.md section 8: Turbopack's 2.4x faster build is not available here until whatever breaks it on Vercel is understood. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Pinning --webpack did not fix the preview deploy, so the builder is not implicated and there is no reason to give up Turbopack's 2.4x faster build (measured, both sides, in docs/PNPM-MIGRATION.md section 8). Parking the Vercel failure here rather than continuing to guess. Four hypotheses were each tested by pushing them alone, and each was wrong: 1. corepack not enabled -> restored ENABLE_EXPERIMENTAL_COREPACK=1 2. engines.node semver range -> back to the "22.x" major selector 3. pnpm-workspace packages: [.] -> removed the monorepo signal 4. Turbopack failing on Vercel -> pinned --webpack Changes 1-3 are correct regardless and are kept; 4 is reverted here. What is known: main and PR #267 both deploy successfully, and every commit on this branch fails, so the cause is on this branch. GitHub Actions runs the same install and the same production build on ubuntu and passes in ~60s, so it is specific to the Vercel environment rather than to pnpm or the build. What is needed: the build log, which cannot be read from here -- there is no Vercel CLI and no token in this environment. One command answers it: npx vercel login && npx vercel inspect <deployment-url> --logs Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Sweeps every tracked file that told a human or an agent to run `yarn`, and adds docs/PNPM-MIGRATION.md as the single write-up of what happened. docs/PNPM-MIGRATION.md is the substantive one. It covers what PnP was doing and what replaced it, the three places pnpm 11 configuration can silently not work, why the install-script allowlist is a security improvement rather than a chore, the override rule the repo learned the hard way, everything the migration exposed, measured before/after numbers, and the history-rewrite runbook with its blast radius. Rewritten rather than substituted: - docs/Tooling.md: the entire "Yarn Plug'n'Play" chapter is replaced with a pnpm chapter, and the git-hygiene table no longer tells you to commit `.pnp.cjs` and `.yarn/sdks`. The PnP troubleshooting section becomes troubleshooting for the two failures that actually occur now -- phantom dependencies and ERR_PNPM_IGNORED_BUILDS. - README.md: the "Package Manager: Executive Recommendation" section argued for exactly this migration, so it is replaced by a short account of what the migration bought, with the reasoning moved into the migration doc. Its central claim is worth preserving and is why the section is not simply deleted: **Turbopack will never support Yarn PnP**, so the App Router work was blocked behind this change. - CLAUDE.md: the standing warning "Never run `yarn test` -- it is Jest watch mode" is gone, because `pnpm test` is now the non-interactive run. The watcher is `pnpm test:watch`, and that is what agents are told to avoid. Restores a `lint:mdx` script. It was dropped when `--ext` was removed, but five documents reference it and flat config takes a glob, so `eslint "**/*.mdx"` is the direct equivalent. Verified it runs clean. next.config.js keeps `eslint.ignoreDuringBuilds: true`, now with a comment explaining why. The README asked to "turn it back on"; that TODO is satisfied by lint being its own CI job rather than by the flag. Making `next build` lint again would run ESLint twice per PR and blame the build step for lint failures. README TODO entries completed by this branch are removed rather than left to rot: the pnpm migration itself, the `ls -la .yarn` build prefix, the contradictory `engines.yarn`, `test:ci`, and the missing CI workflow. The `.yarn/cache` history purge stays open and now points at the runbook. Verified: lint, lint:mdx, typecheck and 4/4 tests all clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Does the two things in the order the risk demanded. The previous commit annotated this as a known issue precisely because it is a medical dosing calculator with no test coverage; this adds the coverage first and only then touches the component. 1. `calculateTotals` is extracted into utils/calculations.ts **verbatim**. No arithmetic changed. It exists so the numbers can be asserted without rendering. 2. 22 characterisation tests pin the behaviour. They are explicitly NOT a clinical review -- they assert what the calculator does today so that a refactor can prove it moved nothing. 3. The component drops `morphineEq`/`methadoneEq` state and the effect that re-synced them, and computes with `useMemo` during render instead. That was a second render pass on every keystroke. The eslint-disable added earlier is gone with it. The tests were mutation-checked rather than merely observed passing. Flipping the Methadone branch from squaring to multiplying turns 3 red; deriving methadone from the rounded rather than the unrounded total turns 4 red. A suite never seen to fail is not evidence. Writing them surfaced three behaviours that are PINNED, NOT ENDORSED. Each looks like a bug, none is changed, because whether they are bugs is a clinical question: - **Methadone is special-cased to dose squared**, not `dose * toMorphine`. Methadone's potency really is non-linear in daily dose, so a special case is expected -- but this particular curve is unverified. - **Methadone's `toMorphine: 0.25` is dead data.** The squaring branch never reads it, so editing that number has no effect whatsoever. A test asserts this, so wiring it back in fails loudly instead of silently changing every methadone conversion. - **The two outputs round inconsistently.** `morphineEq` is the rounded total; `methadoneEq` derives from the UNROUNDED total. At 3 mg of codeine the UI shows 0 mg morphine equivalent beside 1 mg methadone equivalent. Also: the branch matches `display === 'Methadone'` exactly, so any other spelling silently takes the multiply path. None of the above is a regression introduced here -- all of it predates this branch and is now visible instead of implicit. Verified: lint, typecheck, 22/22 tests, production build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
A fresh Opus agent was briefed to refute this work rather than approve it. It found real defects, including three claims I had made that were wrong. Fixing the substantive ones and correcting the false statements. **Advisories: 5 -> 0.** docs/PNPM-MIGRATION.md claimed the five remaining advisories had "no upstream fix." That was false. Both packages had published fixes and this project already depended on fixed versions directly; the vulnerable copies (postcss@8.4.31, sharp@0.34.5) were dragged in transitively by next. The fix is the technique §5 of that same document teaches -- override the version so the old transitive copy cannot resolve -- applied to the one case in the tree where it clears everything. `pnpm audit` now reports no known vulnerabilities. Verified against the lockfile (zero references to either old version) and by resolving postcss from inside next, which sees 8.5.26. Lint, typecheck, tests and a production build all pass with the overrides in place. **The pre-commit hook was reporting success for changes it never tested.** next/jest does not translate tsconfig `paths` into a Jest moduleNameMapper. Runtime resolution survives through the SWC transform, but Jest's static dependency graph does not, so `jest --findRelatedTests` returned ZERO matches for every @/-aliased module -- including the dosing calculator this branch exists to protect. lint-staged runs that with `--passWithNoTests`, which turns "found no tests" into a green tick. Adding the mapper takes `--findRelatedTests calculations.ts` from 0 tests to 18. **Coverage was flattering by 3.7x.** With no `collectCoverageFrom`, coverage was computed over the 14 modules some test happened to import, out of 41. OpioidConverter.tsx -- the file this branch refactored -- was absent from the table entirely. Configured properly, the honest number is 13.47%, not 49.33%. **CI could pass a commit to main with no completed run.** Both workflows set `cancel-in-progress: true` on pushes to main. A cancelled run is not a failure and shows no red X, so two merges landing close together could leave a commit on main ungated. Cancellation is now conditioned off the default branch. **Two documentation files were never committed,** including src/resources/LLMPrompts.mdx, which RENDERS ON THE LIVE SITE and still told readers to run `yarn install` against a yarn.lock this branch deletes. The previous docs commit claimed to sweep every yarn reference; it missed these. Corrections to claims made earlier on this branch: - The mutation evidence in §7.1 was overstated. The second mutant turns **1 test red, not 4**. The original measurement was taken while the first mutant was still applied, because the restore step had silently failed, so three of those four failures belonged to the other mutation. This matters: the rounding asymmetry is pinned by a single assertion pair, because every other test uses integer totals where rounding is a no-op. - "22 tests pin it" was the whole-repo count. The module has 18. - §2 claimed parity with `pnpMode: strict` on phantom imports. That holds at the project root but not inside the store: pnpm's default `hoistPattern: ["*"]` hoists 787 packages where any package can reach them. `hoistPattern: []` would restore full strictness; it is recorded as a known gap rather than changed here, because it alters resolution for every package in the tree and needs its own verification pass. Not addressed here, and left in the review's own words in HANDOFF.md: the NaN coercion path in the converter's text input, two contradictory drug tables (OPIOID_OPTIONS is dead code and disagrees with MEDICATION_ARRAY), and three weak assertions in the new suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
The adversarial review flagged that `handleDoseChange` does `Number(value)` on a `type="text"` input, so anything typeable reaches the arithmetic, and none of it had coverage. Six tests pin what currently happens. All six document behaviour that is wrong-looking, and none is changed here -- the fix belongs at the input, not in the arithmetic, and it is a UI decision: - Typing a letter yields NaN, which survives rounding and the sum, so the component renders "Morphine Equivalence: NaN mg". - One bad field poisons the whole total, discarding every other dose entered. - `0x10` is silently accepted as 16 and `5e3` as 5000 -- three orders of magnitude from what was typed. - A negative dose is accepted and produces a NaN methadone equivalent, because sqrt of a negative is NaN. - An empty field coerces to 0, which is the one sensible case. Pinned rather than fixed so that whoever repairs the input has a concrete failing-test target and can see exactly which behaviours move. 28 tests, lint and typecheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013RtsQWaEaAe4iVnSoUCDfY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A package-manager migration that turned into a quality-gate repair, because the migration is what made the broken gates visible.
The headline
pnpm lintcrashed, one Jest test failed, and coverage had never run — all onmain, before this branch. Verified pre-existing by runningyarn lintagainst the pre-migration checkout, where it fails the same way.They survived because the repo had exactly one CI workflow and it audited dependencies. No job ever ran lint, tests, or a build.
Commits
dd651c94cda87f0202086f4f441e083c66135c53dfdocs/PNPM-MIGRATION.mdff861f8The migration
pnpm importbuilt the lockfile fromyarn.lock, preserving every resolution — so commit 01 changes how packages arrive and nothing about which.1,379 committed
.yarn/cachezips are gone (778 MB of working tree), along with.pnp.cjs, the.yarn/sdkseditor shims, and the.gitignorerules excluding*-win32-*/*-darwin-*archives that existed only because installing on Windows would otherwise overwrite the linux binaries Vercel needed.docs/PNPM-MIGRATION.md§9 — documentation only, since it rewrites history and invalidates every open branch.pnpm 11 specifics that fail silently
.npmrcis auth/registry-only now; everything else lives inpnpm-workspace.yamlallowBuildsreplacedonlyBuiltDependencies, which pnpm 11 removed and now ignores — config from any pnpm 10 guide looks right and does nothingminimumReleaseAgedefaults to 1440, so packages published in the last 24h will not resolveA security improvement, not just a swap
Yarn ran every dependency install script (
enableScripts: true). pnpm denies all by default; four packages are allowlisted with reasons inline. Everything else gets no install-time code execution.Review risks, ranked
mainand Migrate Pages Router to App Router #267 deploy fine; every commit here fails. GitHub Actions runs the same install and build on ubuntu and passes in ~50s, so it is Vercel-specific. Four hypotheses were each pushed alone and all four were wrong (details below). I stopped rather than guess a fifth time.npx vercel login && npx vercel inspect <url> --logsanswers it.eslint-plugin-react's latest release declareseslint: "^3 || … || ^9.7"— no ESLint 10 support at all, andeslint-config-nextpulls it in transitively. The repo had moved ahead of its plugin ecosystem. Falsifier: revisit when that plugin ships an ESLint 10 peer range.yarn.lockdiffs and cannot rebase onto a tree with noyarn.lock. Their substance is folded into commit 05. Closing 20 PRs is yours to do.The dosing calculator
Commit 08 did the two things in the order the risk demanded: characterisation tests first, refactor second.
calculateTotalswas extracted verbatim, 22 tests pin it, and the component now computes withuseMemoinstead of syncing state through an effect.The tests were mutation-checked, not merely observed passing: flipping the Methadone branch from squaring to multiplying turns 3 red; deriving methadone from the rounded rather than unrounded total turns 4 red.
Writing them surfaced three behaviours that are pinned, not endorsed. None was changed:
toMorphine: 0.25is dead data. The squaring branch never reads it, so editing that number has no effect. A test now asserts this.display === 'Methadone'exactly. Any other spelling silently takes the multiply path.Turbopack is a trade, not a win
Two runs each,
.nextdeleted between, ±10 ms:2.4× faster, +25 KB gzipped. This branch keeps Turbopack. Note #267 pins
--webpackand pays the 2.4× — and the reason it couldn't use Turbopack is the same reason this migration was blocking: Turbopack will never support Yarn PnP, by design.The four Vercel hypotheses, and why each was rejected
ENABLE_EXPERIMENTAL_COREPACK=1. Still failed. Kept; correct anyway.engines.nodesemver range — Vercel wants a major selector, so back to"22.x". Still failed. Kept; correct anyway.pnpm-workspace.yamlpackages: [.]making a single-package repo look like a monorepo. Removed, verified overrides and allowed builds still apply. Still failed. Kept; correct anyway.--webpack. Pinned it here too; still failed, so the builder is not implicated. Reverted.Verified
pnpm install --frozen-lockfile14.9s cold · lint clean · typecheck clean · 26/26 tests with coverage · production build 12.5s including service worker and sitemap · both GitHub Actions workflows green.